Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix translations_path #2521

Merged
merged 3 commits into from
Sep 26, 2024
Merged

fix translations_path #2521

merged 3 commits into from
Sep 26, 2024

Conversation

LeXofLeviafan
Copy link
Contributor

The translations_path constructor parameter appears to have never actually worked with flask-babel, because flask_babel.Domain does not have a get_translations_path() method to override (unlike flask-babelex).

samuelhwilliams
samuelhwilliams previously approved these changes Sep 15, 2024
View reviewed changes
Copy link
Contributor

@samuelhwilliams samuelhwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you - good catch. We definitely need some better tests around some of these areas - could you raise an issue to track that (and/or if you're willing to write some tests around babel, that would be very useful) 🙏

@samuelhwilliams
Copy link
Contributor

Would you mind also adding a line to docs/changelog.rst recording this bug fix?

Along the lines of:

2.0.0a1
-------

Fixes:

* Fix `translations_path` attribute when Flask-Admin is used with Flask-Babel.

@LeXofLeviafan
Copy link
Contributor Author

I've added a test for translations_path… though it's rather low-level 😅 (The upside is that it doesn't require doing actual environment setup as it only checks whether the specified directory gets used when building the translations cache.)

As for raising an issue – I suspect you'd know how to word it better (and what else needs to be tested) as you're more familiar with the codebase 🤔

@samuelhwilliams samuelhwilliams merged commit e20a213 into pallets-eco:master Sep 26, 2024
11 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@samuelhwilliams samuelhwilliams samuelhwilliams approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@LeXofLeviafan @samuelhwilliams